Fix MBOMsg wire layout (order_id/price/ts_recv offsets) and StatMsg undefined-quantity sentinel - #46
Merged
Merged
Conversation
read_mbo_msg / write_record(::MBOMsg) used a field order that does not match the DBN spec (ts_recv at 16, order_id at 24, price at 40). The official MboMsg (identical in v1/v2/v3) has order_id at 16, price at 24, ts_recv at 40. The encoder mirrored the decoder, so Julia round trips and all self-consistency tests passed, but every Databento-produced MBO file decoded with the three fields rotated. Both paths now use the official layout; test_phase5.jl's fixture assertions (which had pinned the swapped values) are corrected and test_mbo_wire_layout.jl decodes Databento's own fixtures against reference values plus a byte-exact re-encode. The StatMsg encoder wrote an undefined v3 quantity as 0xffffffffffffffff (-1) instead of typemax(Int64); the v3 decoder read a UInt64 and mapped every value >= 0x7fff... (any negative) to UNDEF, which masked it. Quantity is now written and read as a signed Int64 (test_stat_quantity_sentinel.jl). Found while generating DuckDB test fixtures for duckdb-dbn, whose reader (databento-cpp layout) disagreed with this package on both points. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBr6EZa1vHrJmPBoecSjRa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two wire-format bugs, found while generating DuckDB fixtures for duckdb-dbn with this package's encoder (the DuckDB reader, which uses the databento-cpp struct layout, disagreed with us on both — and it was right).
1.
MBOMsgfield offsets were swapped (decode and encode)read_mbo_msgreadts_recvfrom bytes 16–23,order_idfrom 24–31 andpricefrom 40–47. The officialMboMsg(dbn crate; identical in DBN v1/v2/v3) isorder_id @16, price @24, size @32, flags @36, channel_id @37, action @38, side @39, ts_recv @40, ts_in_delta @48, sequence @52— i.e. struct order == wire order.Because
write_record(::MBOMsg)mirrored the decoder, Julia round trips were byte-stable and every self-consistency test passed. But any MBO file produced by Databento decoded with the three fields rotated. On Databento's owntest_data.mbo.dbn:order_idpricets_recvtest/test_phase5.jlhad pinned the swapped values (r1.order_id == 3722750000000); corrected. Newtest/test_mbo_wire_layout.jldecodes everytest_data.mbo.*fixture against reference values, checks a byte-exact re-encode of the uncompressed fixtures, and decodes a hand-laid-out record.Impact: any consumer of MBO
price/order_id/ts_recvthrough this package or DatabentoAPI.jl's Live client got rotated values. Files captured by decode→re-encode (DatabentoAPIstream_to_file) are byte-identical to the wire and decode correctly after this fix. Files written from Julia-built records (write_dbn,DBNStreamWriter,csv_to_dbn/json_to_dbn/parquet_to_dbn) carry the swapped layout on disk — see the CHANGELOG migration note.2.
StatMsgundefinedquantitywritten as-1The encoder wrote an undefined v3 quantity (
typemax(Int64)) as0xffffffffffffffff(= -1) instead of the spec sentineltypemax(Int64)(0x7fff…). The v3 decoder read aUInt64and mapped every value>= 0x7fff…— i.e. any negative quantity — to UNDEF, which masked the bug in round trips; other readers showed -1 where they should show NULL/NaN. Quantity is now written and read as a signedInt64. Newtest/test_stat_quantity_sentinel.jl.Tests
Independent of #45 (both branch from
main) and no overlap insrc/. Both PRs do add aninclude(...)line at the same spot intest/runtests.jland an## [Unreleased]section at the top ofCHANGELOG.md, so whichever merges second needs a trivial conflict resolution (keep both). Line endings preserved (git diff --stat==--ignore-cr-at-eol).🤖 Generated with Claude Code
https://claude.ai/code/session_01YBr6EZa1vHrJmPBoecSjRa